server/lib/notifications/agents/webpush.ts (1)

128-139: 💤 Low value

Redundant null check on payload.request.

The condition payload.request && payload.request?.declineReason is redundant. Since the second operand uses optional chaining, it will safely evaluate to undefined if payload.request is nullish. The email agent (line 172) uses simply if (!payload.request.declineReason).

Suggested simplification

       case Notification.MEDIA_DECLINED:
-        if (payload.request && payload.request?.declineReason) {
+        if (payload.request?.declineReason) {
           message = intl.formatMessage(messages.declinedWithReason, {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/lib/notifications/agents/webpush.ts` around lines 128 - 139, The
condition checking `payload.request && payload.request?.declineReason` contains
a redundant null check. Since optional chaining (the `?.` operator) already
safely handles nullish values by returning undefined, the explicit
`payload.request &&` check is unnecessary. Simplify the if condition to just `if
(payload.request?.declineReason)` to remove the redundancy while maintaining the
same behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/components/RequestModal/DeclineRequestModal/index.tsx (1)

50-58: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Re-throw request failures so batch decline can’t report false success.

Line 50-58 catches axios.post errors but resolves instead of rejecting; then Line 92-94 never detects failures and Line 98-100 may still call onComplete() after partial/failed declines.

Suggested fix

   const handleSubmit = async (
     requestId: number,
     values: { reason: string }
   ) => {
     try {
       await axios.post(`/api/v1/request/${requestId}/decline`, {
         declineReason: values.reason,
       });
-    } catch {
-      if (onError) {
-        onError();
-      }
+    } catch (error) {
+      onError?.();
+      throw error;
     }
   };

Also applies to: 88-95

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/RequestModal/DeclineRequestModal/index.tsx` around lines 50 -
58, The catch block in the axios.post call for declining a request (lines 50-58)
handles the error by calling the onError callback but does not re-throw the
error, causing the Promise to resolve successfully even when the request fails.
This prevents the batch decline logic from detecting failures, allowing false
success reports. After calling onError() in the catch block, re-throw the error
so that the calling code can properly detect and handle request failures.

src/components/RequestButton/index.tsx (1)

166-166: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Persist decline targets in state; current wiring can decline the wrong requests.

Line 166/194/236/264 only toggles modal visibility. At Line 399, modal requests are always derived from non-4K state (activeRequest/activeRequests), so 4K decline actions can operate on the wrong set. Also, Line 396 prevents rendering when only 4K requests are pending.

Suggested fix

+  const [declineTargets, setDeclineTargets] = useState<MediaRequest[]>([]);
@@
           action: () => {
+            setDeclineTargets([activeRequest]);
             setShowDeclineCommentModal(true);
           },
@@
           action: () => {
+            setDeclineTargets(activeRequests);
             setShowDeclineCommentModal(true);
           },
@@
           action: () => {
+            setDeclineTargets([active4kRequest]);
             setShowDeclineCommentModal(true);
           },
@@
           action: () => {
+            setDeclineTargets(active4kRequests);
             setShowDeclineCommentModal(true);
           },
@@
-      {(activeRequest || (activeRequests && activeRequests.length > 0)) && (
+      {declineTargets.length > 0 && (
         <DeclineRequestModal
           show={showDeclineCommentModal}
-          requests={activeRequest ? [activeRequest] : activeRequests}
+          requests={declineTargets}
           type={mediaType}
-          onCancel={() => setShowDeclineCommentModal(false)}
+          onCancel={() => {
+            setShowDeclineCommentModal(false);
+            setDeclineTargets([]);
+          }}
           onComplete={() => {
             onUpdate();
             mutate('/api/v1/request/count');
             setShowDeclineCommentModal(false);
+            setDeclineTargets([]);
           }}
         />
       )}

Also applies to: 194-194, 236-236, 264-264, 396-400

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/RequestButton/index.tsx` at line 166, The decline modal
visibility toggles at lines 166, 194, 236, and 264 do not persist which specific
request is being declined. The modal confirmation handler at line 399 always
uses the generic activeRequest or activeRequests state to determine what to
decline, which causes the wrong request to be declined when a 4K request decline
action opens the modal. Add a new state variable to track the specific decline
target (either a single request or list of requests), populate it alongside
setShowDeclineCommentModal(true) at each of the four toggle locations, and then
use this persisted decline target in the modal confirmation handler instead of
dereferencing activeRequest or activeRequests.

server/migration/sqlite/1781992375456-AddRequestDeclinedMessage.ts (1)

7-20: ⚡ Quick win

Remove redundant user_push_subscription rebuilds.

The user_push_subscription table is rebuilt twice in up() (lines 7-20 and 47-60) with identical schema each time. Since this migration only adds declineReason to media_request, these operations are unnecessary and appear to be TypeORM migration generator artifacts. They double migration time and add DDL risk without effect.

Same issue exists in down() at lines 64-77 and 104-117.

♻️ Suggested cleanup for up() method

 public async up(queryRunner: QueryRunner): Promise<void> {
-  await queryRunner.query(`DROP INDEX "IDX_03f7958328e311761b0de675fb"`);
-  await queryRunner.query(
-    `CREATE TABLE "temporary_user_push_subscription" ("id" integer PRIMARY KEY AUTOINCREMENT NOT NULL, "endpoint" varchar NOT NULL, "p256dh" varchar NOT NULL, "auth" varchar NOT NULL, "userId" integer, "userAgent" varchar, "createdAt" datetime DEFAULT (CURRENT_TIMESTAMP), CONSTRAINT "UQ_6427d07d9a171a3a1ab87480005" UNIQUE ("endpoint", "userId"), CONSTRAINT "UQ_f90ab5a4ed54905a4bb51a7148b" UNIQUE ("auth"), CONSTRAINT "FK_03f7958328e311761b0de675fbe" FOREIGN KEY ("userId") REFERENCES "user" ("id") ON DELETE CASCADE ON UPDATE NO ACTION)`
-  );
-  await queryRunner.query(
-    `INSERT INTO "temporary_user_push_subscription"("id", "endpoint", "p256dh", "auth", "userId", "userAgent", "createdAt") SELECT "id", "endpoint", "p256dh", "auth", "userId", "userAgent", "createdAt" FROM "user_push_subscription"`
-  );
-  await queryRunner.query(`DROP TABLE "user_push_subscription"`);
-  await queryRunner.query(
-    `ALTER TABLE "temporary_user_push_subscription" RENAME TO "user_push_subscription"`
-  );
-  await queryRunner.query(
-    `CREATE INDEX "IDX_03f7958328e311761b0de675fb" ON "user_push_subscription" ("userId") `
-  );
   await queryRunner.query(`DROP INDEX "IDX_4c696e8ed36ae34fe18abe59d2"`);
   // ... media_request rebuild with declineReason ...
   await queryRunner.query(
     `CREATE INDEX "IDX_4c696e8ed36ae34fe18abe59d2" ON "media_request" ("status") `
   );
-  await queryRunner.query(`DROP INDEX "IDX_03f7958328e311761b0de675fb"`);
-  // ... second identical user_push_subscription rebuild ...
 }

Also applies to: 47-60

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/migration/sqlite/1781992375456-AddRequestDeclinedMessage.ts` around
lines 7 - 20, The migration file contains duplicate and identical rebuilds of
the user_push_subscription table in both the up() and down() methods. In the
up() method, remove the second identical user_push_subscription rebuild sequence
(lines 47-60) that duplicates the first one (lines 7-20), and in the down()
method, remove the second identical sequence (lines 104-117) that duplicates the
first one (lines 64-77). Since this migration only adds declineReason to
media_request, the duplicate user_push_subscription table operations are
unnecessary and should be eliminated to reduce migration time and DDL risk.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes